Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint YAML files (mainly semantic conventions). #1814

Merged
merged 4 commits into from
Jul 15, 2021

Conversation

Oberon00
Copy link
Member

@Oberon00
Copy link
Member Author

It even generates annotations on the diff page:

image

run: make install-yamllint

- name: run yamllint
run: yamllint . -f github
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using make here because of the -f github special option.

@Oberon00 Oberon00 marked this pull request as ready for review July 13, 2021 12:05
@Oberon00 Oberon00 requested review from a team July 13, 2021 12:05
.yamllint Show resolved Hide resolved
@arminru arminru added the editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind. label Jul 13, 2021
@SergeyKanzhelev
Copy link
Member

Should it be mentioned in CONTRIBUTING.md? Maybe tools that can be used to auto-format?

Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to update contributing.md as well

@Oberon00
Copy link
Member Author

Maybe tools that can be used to auto-format?

I was looking for such a tool but couldn't quickly find one, at least not one that worked on Windows where I tried.

@Oberon00
Copy link
Member Author

Oberon00 commented Jul 14, 2021

@SergeyKanzhelev

Should it be mentioned in CONTRIBUTING.md? Maybe tools that can be used to auto-format?

Good point, it would also be helpful to have a Make target that executes all linters. But maybe in a follow-up PR?

Also we should consider making a few more checks required, at least the old semantic convention check. I think this must be done by an admin in the repo settings.

@bogdandrutu
Copy link
Member

@arminru since you have magic power to upgrade this branch, please upgrade it then merge the PR 🗡️

@arminru arminru merged commit d872294 into open-telemetry:main Jul 15, 2021
@arminru arminru deleted the yamllint branch July 15, 2021 08:12
@arminru
Copy link
Member

arminru commented Jul 15, 2021

Merged and also added yamllint as a required check for main.

carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants